Skip to content

change: [UIE-9888] - Make firewall selection mandatory while creating linode and its interfaces#13410

Open
grevanak-akamai wants to merge 6 commits intolinode:developfrom
grevanak-akamai:feature/UIE-10132-block-interface-creation-without-firewall
Open

change: [UIE-9888] - Make firewall selection mandatory while creating linode and its interfaces#13410
grevanak-akamai wants to merge 6 commits intolinode:developfrom
grevanak-akamai:feature/UIE-10132-block-interface-creation-without-firewall

Conversation

@grevanak-akamai
Copy link
Contributor

@grevanak-akamai grevanak-akamai commented Feb 18, 2026

Description 📝

Make firewall selection mandatory while creating linode and its interfaces. If there are no firewalls or user doesn't wants to assign a firewall to a linode, "No firewall" option can be selected but the firewall field cannot be left blank.

Changes 🔄

List any change(s) relevant to the reviewer.

  • Firewall selection is made mandatory while creating linode and adding interfaces to linode.
  • Added new option "No firewalls" under select firewall dropdown to allow users to not assign a firewall(Not recommended though). Added a warning notice if "No firewall" option is selected.
  • Updated linode schema to make firewall field mandatory.

Scope 🚢

Upon production release, changes in this PR will be visible to:

  • All customers
  • Some customers (e.g. in Beta or Limited Availability)
  • No customers / Not applicable

Target release date 🗓️

Mar 2026

Preview 📷

1. If no firewall is selected
Screenshot 2026-02-23 at 1 47 43 PM
2. "No firewall" option in the dropdown
Screenshot 2026-02-23 at 1 48 18 PM

How to test 🧪

Prerequisites

  • Navigate to Create linode page.

Verification steps

  • Ensure create linode submission fails if no option is selected under firewall dropdown.
  • Select any firewall or select "No Firewall" option in the dropdown and ensure linode is created/cloned.
Author Checklists

As an Author, to speed up the review process, I considered 🤔

👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support


  • I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

  • All tests and CI checks are passing
  • TypeScript compilation succeeded without errors
  • Code passes all linting rules

@grevanak-akamai grevanak-akamai force-pushed the feature/UIE-10132-block-interface-creation-without-firewall branch from c096c03 to 4844424 Compare February 18, 2026 11:07

export const CreateLinodeInterfaceSchema = object({
firewall_id: number().nullable(),
firewall_id: number()
Copy link
Contributor Author

@grevanak-akamai grevanak-akamai Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API accepts both -1 and null(will be deprecated soon and will be utilized only to assign default firewall if any) to not to assign a firewall to a linode and hence firewall field has been made mandatory in UI.

@grevanak-akamai grevanak-akamai force-pushed the feature/UIE-10132-block-interface-creation-without-firewall branch from 3926c9e to 262df83 Compare February 22, 2026 18:18
label="Linode Interfaces - Public Interface Firewall"
onChange={(e, firewall) => field.onChange(firewall.id)}
placeholder={DEFAULT_FIREWALL_PLACEHOLDER}
showNoFirewallOption={false}
Copy link
Contributor Author

@grevanak-akamai grevanak-akamai Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In future, default firewalls will also support "No Firewalls" option.

@grevanak-akamai grevanak-akamai marked this pull request as ready for review February 23, 2026 15:21
@grevanak-akamai grevanak-akamai requested review from a team as code owners February 23, 2026 15:21
@grevanak-akamai grevanak-akamai requested review from bnussman-akamai, coliu-akamai and jdamore-linode and removed request for a team February 23, 2026 15:21
@grevanak-akamai grevanak-akamai self-assigned this Feb 23, 2026
@grevanak-akamai grevanak-akamai added Ready for Review Linodes Dealing with the Linodes section of the app labels Feb 23, 2026
@grevanak-akamai
Copy link
Contributor Author

Cloud Manager UI test results

🔺 3 failing tests on test run #5 ↗︎

❌ Failing ✅ Passing ↪️ Skipped 🕐 Duration
3 Failing 865 Passing 11 Skipped 40m 58s

Details

Failing Tests
Spec Test
search-images.spec.ts Cloud Manager Cypress Tests→Search Images » creates two images and make sure they show up in the table and are searchable
account-switching.spec.ts Cloud Manager Cypress Tests→Parent/Child account switching→From Parent to Child » can search child accounts
smoke-community-stackscripts.spec.ts Cloud Manager Cypress Tests→Community Stackscripts integration tests » deploys a new linode as expected

Troubleshooting

Use this command to re-run the failing tests:

pnpm cy:run -s "cypress/e2e/core/images/search-images.spec.ts,cypress/e2e/core/parentChild/account-switching.spec.ts,cypress/e2e/core/stackscripts/smoke-community-stackscripts.spec.ts"

Failure in smoke-community-stackscripts.spec.ts is related I guess. Rest 2 are unrelated.

@grevanak-akamai grevanak-akamai force-pushed the feature/UIE-10132-block-interface-creation-without-firewall branch from 5724cc4 to b700773 Compare February 25, 2026 13:13
@grevanak-akamai grevanak-akamai force-pushed the feature/UIE-10132-block-interface-creation-without-firewall branch from b700773 to b5ffc6d Compare February 26, 2026 15:45
Copy link
Contributor

@coliu-akamai coliu-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @grevanak-akamai! Had a couple of questions/comments below.

Also, this PR is missing a changeset - do we still need changesets for the release process? looked at some other open PRs and they had changesets - could we add one to this PR?

Comment on lines +12 to +14
firewall_id: number().required(
'Select an option or create a new Firewall.'
),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just have this part say 'Select an option.' only? There's no option to create a firewall on the Add Interface drawer:

Image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree but user can create a firewall from firewalls page if no option is available. So suggesting this message. I can check with our techwriter too.

values.linodeInterfaces = values.linodeInterfaces.map(
getCleanedLinodeInterfaceValues
);
if (tab === 'Clone Linode' && !values.firewall_id) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lines 41-43 look the same as lines 72-75 - do we need this check here underneath if (context?.isLinodeInterfacesEnabled) if we're running it anyway at lines 72-75?

Also, regarding this comment:
The Clone Linode flow does not have the firewall_id field under interfaces object, so we set firewall_id to -1 to bypass the firewall requirement in the validation schema.

just to double check - since we're keeping the original firewall schema which lets us have undefined values, do we still need this condition & the ones below?

Copy link
Contributor Author

@grevanak-akamai grevanak-akamai Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree lines 41-43 is redundant so removed it. However we still need conditions to override firewall_id as we are making it mandatory in CreateLinodeInterfaceSchema. Let me know if I'm missing something here.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enforces an explicit firewall choice during Linode creation and when adding Linode interfaces by introducing a “No firewall” selectable option (represented by firewall_id = -1) and showing a warning notice when that option is chosen.

Changes:

  • Make firewall selection required in Linode Create (legacy config) and Add Interface flows, while still allowing an explicit “No firewall” selection.
  • Extend FirewallSelect to include a “No firewall (not recommended)” option and an optional warning notice message.
  • Update unit tests and a broad set of Cypress E2E flows to select a firewall (or “No firewall”) to satisfy the new requirement.

Reviewed changes

Copilot reviewed 45 out of 45 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
packages/validation/src/linodes.schema.ts Adjusts Create Linode schema firewall field definition (nullable).
packages/manager/src/features/VPCs/VPCDetail/SubnetAssignLinodesDrawer.tsx Passes “No firewall” warning message into firewall selection for VPC interface assignment.
packages/manager/src/features/Linodes/constants.ts Adds shared warning message string for “No firewall” selections.
packages/manager/src/features/Linodes/LinodesDetail/LinodeNetworking/LinodeInterfaces/EditInterfaceDrawer/EditInterfaceFirewall.tsx Disables “No firewall” option in edit-interface firewall selection.
packages/manager/src/features/Linodes/LinodesDetail/LinodeNetworking/LinodeInterfaces/AddInterfaceDrawer/utilities.ts Makes firewall_id required in Add Interface form schema.
packages/manager/src/features/Linodes/LinodesDetail/LinodeNetworking/LinodeInterfaces/AddInterfaceDrawer/InterfaceType.tsx Sets firewall_id to -1 for VLAN interfaces (no firewall support).
packages/manager/src/features/Linodes/LinodesDetail/LinodeNetworking/LinodeInterfaces/AddInterfaceDrawer/InterfaceFirewall.tsx Updates placeholder and passes warning message prop to FirewallSelect.
packages/manager/src/features/Linodes/LinodesDetail/LinodeNetworking/LinodeInterfaces/AddInterfaceDrawer/AddInterfaceForm.tsx Changes default firewall value handling and adjusts validation resolver inputs.
packages/manager/src/features/Linodes/LinodeCreate/utilities.test.tsx Updates tests to include firewall_id in interface fixtures.
packages/manager/src/features/Linodes/LinodeCreate/schemas.ts Adds conditional firewall requirement for legacy config create flow.
packages/manager/src/features/Linodes/LinodeCreate/resolvers.ts Adds -1 sentinel assignments to satisfy validation (incl. Clone flow handling).
packages/manager/src/features/Linodes/LinodeCreate/Summary/Summary.tsx Treats firewall_id = -1 as “no firewall” when computing “Firewall Assigned”.
packages/manager/src/features/Linodes/LinodeCreate/Networking/InterfaceType.tsx Sets per-interface firewall_id to -1 for VLAN interface type.
packages/manager/src/features/Linodes/LinodeCreate/Networking/InterfaceFirewall.tsx Updates placeholder and passes warning message prop to FirewallSelect.
packages/manager/src/features/Linodes/LinodeCreate/Networking/Firewall.tsx Updates placeholder and passes warning message prop to FirewallSelect.
packages/manager/src/features/Linodes/LinodeCreate/Firewall.tsx Updates placeholder and passes warning message prop to FirewallSelect.
packages/manager/src/features/Kubernetes/NodePoolFirewallSelect.tsx Hides “No firewall” option in Kubernetes node pool firewall selection.
packages/manager/src/features/Firewalls/components/FirewallSelect.tsx Adds “No firewall” option, warning notice rendering, and related props.
packages/manager/src/features/Firewalls/components/FirewallSelect.test.tsx Adds unit tests for “No firewall” option and warning notice behavior.
packages/manager/src/features/Account/DefaultFirewalls.tsx Hides “No firewall” option in account default firewall selection fields.
packages/manager/cypress/support/util/linodes.ts Updates test Linode creation payload to use firewall_id = -1.
packages/manager/cypress/support/ui/pages/linode-create-page.ts Adds a reusable Cypress helper to select a firewall from the dropdown.
packages/manager/cypress/e2e/core/stackscripts/smoke-community-stackscripts.spec.ts Mocks firewalls and selects one during Linode creation flow.
packages/manager/cypress/e2e/core/stackscripts/create-stackscripts.spec.ts Mocks firewalls and selects firewall / “No firewall” in create flows.
packages/manager/cypress/e2e/core/placementGroups/create-linode-with-placement-groups.spec.ts Mocks and selects firewall as part of placement group creation scenarios.
packages/manager/cypress/e2e/core/oneClickApps/one-click-apps.spec.ts Mocks and selects firewall in Marketplace/one-click app create flow.
packages/manager/cypress/e2e/core/linodes/create-linode.spec.ts Mocks and selects firewall across multiple Linode create scenarios.
packages/manager/cypress/e2e/core/linodes/create-linode-with-vpc.spec.ts Mocks and selects firewall in VPC-related Linode create flows.
packages/manager/cypress/e2e/core/linodes/create-linode-with-vlan.spec.ts Mocks and selects firewall in VLAN-related Linode create flows.
packages/manager/cypress/e2e/core/linodes/create-linode-with-user-data.spec.ts Mocks and selects firewall for user-data Linode creation.
packages/manager/cypress/e2e/core/linodes/create-linode-with-ssh-key.spec.ts Mocks and selects firewall for SSH-key Linode creation scenarios.
packages/manager/cypress/e2e/core/linodes/create-linode-with-firewall.spec.ts Refactors firewall selection steps to use the shared page helper.
packages/manager/cypress/e2e/core/linodes/create-linode-with-disk-encryption.spec.ts Mocks and selects firewall for disk encryption Linode creation.
packages/manager/cypress/e2e/core/linodes/create-linode-with-add-ons.spec.ts Mocks and selects firewall in add-ons creation flows (incl. legacy selection).
packages/manager/cypress/e2e/core/linodes/create-linode-vm-host-maintenance.spec.ts Mocks and selects firewall in VM host maintenance create flow.
packages/manager/cypress/e2e/core/linodes/create-linode-view-code-snippet.spec.ts Mocks and selects firewall before opening code snippet modal.
packages/manager/cypress/e2e/core/linodes/create-linode-mobile.spec.ts Mocks and selects firewall in mobile viewport create flow.
packages/manager/cypress/e2e/core/linodes/create-linode-in-distributed-region.spec.ts Mocks and selects firewall in distributed region create flow.
packages/manager/cypress/e2e/core/linodes/create-linode-in-core-region.spec.ts Mocks and selects firewall in core region create flow.
packages/manager/cypress/e2e/core/linodes/create-linode-blackwell.spec.ts Mocks and selects firewall in Blackwell GPU create smoke test.
packages/manager/cypress/e2e/core/linodes/clone-linode.spec.ts Mocks and selects firewall during clone-related test flow.
packages/manager/cypress/e2e/core/linodes/alerts-create.spec.ts Mocks and selects firewall in alerts-create related flows.
packages/manager/cypress/e2e/core/images/create-linode-from-image.spec.ts Mocks and selects firewall in “create from image” flow.
packages/manager/cypress/e2e/core/helpAndSupport/open-support-ticket.spec.ts Mocks and selects firewall to satisfy create form in support-ticket scenario.
packages/manager/cypress/e2e/core/general/gdpr-agreement.spec.ts Mocks and selects firewall in GDPR agreement gating flow.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@grevanak-akamai grevanak-akamai force-pushed the feature/UIE-10132-block-interface-creation-without-firewall branch from b5ffc6d to 261781e Compare March 2, 2026 12:10
Copy link
Contributor

@coliu-akamai coliu-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @grevanak-akamai, have a few more comments

regarding the e2e tests - not sure why later commits haven't generated an overview of the failing tests. I looked into the smoke-stackscripts one since you mentioned earlier it's related to your changes tho


// Select a firewall
linodeCreatePage.selectFirewall(
mockFirewall.label,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re: fixing this test, I tried selecting the no firewall option, and it seems to work, at least on my end

Suggested change
mockFirewall.label,
'No firewall - traffic is unprotected (not recommended)',

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. With actual firewall, it is breaking not sure why.

@grevanak-akamai grevanak-akamai force-pushed the feature/UIE-10132-block-interface-creation-without-firewall branch from 261781e to cadb44e Compare March 3, 2026 07:47
@grevanak-akamai
Copy link
Contributor Author

@jdamore-linode - can you test the changes in this PR and also take a look at e2e test cases that are failing in this PR? I'm guessing failing ones are not related to changes in this PR. Test report is somehow not generating after pushing commits.

linodeCreatePage.selectRegionById(linodeRegion.id);
linodeCreatePage.selectPlan('Shared CPU', 'Nanode 1 GB');
linodeCreatePage.setRootPassword(randomString(32));
// Select a firewall
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are selecting firewall twice in one flow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is correct. We are testing both linode interfaces and legacy config profiles in one test case itself. So selecting firewall in both usecases to not break the test flow.

import type { Firewall } from '@linode/api-v4';
import type { EnhancedAutocompleteProps } from '@linode/ui';

const NO_FIREWALL_ID = -1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add comments explaining why -1 is used and how it differs from null.

@@ -137,6 +137,7 @@ export const NodePoolFirewallSelect = (props: NodePoolFirewallSelectProps) => {
}
}}
placeholder="Select firewall"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we standardize the placeholder with other files

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a default placeholder added in the parent component. So I think having another layer of standardization is not required.

@bnussman-akamai
Copy link
Member

I will re-review this today

Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Required firewall seems to be working.

Will continue to test and monitor after this is merged.

const firewallOptions = useMemo(
() => [
...(options ? options : (firewalls ?? [])),
...(showNoFirewallOption ? [noFirewallOption] : []),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a UX standpoint, I worry that it will be a frustrating experience having the "No Firewall" option last in the dropdown.

If a customer has just a few firewalls on their account, they may not even know that "No Firewall" is an option because it won't be in the viewport.

We're adding more friction for users to create Linodes but I guess that's the point of this change

Comment on lines 59 to 61
const valuesWithOnlySelectedInterface = getCleanedLinodeInterfaceValues(
structuredClone(rawValues)
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than making this change, maybe we cast here?

      const valuesWithOnlySelectedInterface = getCleanedLinodeInterfaceValues(
        structuredClone(rawValues)
      ) as CreateInterfaceFormValues;

Copy link
Contributor

@coliu-akamai coliu-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks Ganesh - left a comment below. Also just double checking - is UIE-9888 the right ticket number for this PR? When I navigated to that ticket, I saw something related to nodebalancers

mockGetUserPreferences({ desktop_sidebar_open: false }).as(
'getPreferences'
);
mockGetFirewalls([mockFirewall]).as('getFirewalls');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can remove this, lines 289-292, and the other unused imports if we're only selecting the No Firewalls option - don't need to mock an existing firewall

@github-project-automation github-project-automation bot moved this from Review to Approved in Cloud Manager Mar 3, 2026
@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🔺 3 failing tests on test run #13 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
3 Failing866 Passing11 Skipped36m 5s

Details

Failing Tests
SpecTest
account-switching.spec.tsCloud Manager Cypress Tests→Parent/Child account switching→From Child to Parent » can switch from Proxy user back to Parent account user from Billing page
account-switching.spec.tsCloud Manager Cypress Tests→Parent/Child account switching→From Child to Child » can switch to another Child account as a Proxy user
object-storage.e2e.spec.tsCloud Manager Cypress Tests→object storage end-to-end tests » can create and delete object storage buckets

Troubleshooting

Use this command to re-run the failing tests:

pnpm cy:run -s "cypress/e2e/core/parentChild/account-switching.spec.ts,cypress/e2e/core/objectStorage/object-storage.e2e.spec.ts"

@coliu-akamai
Copy link
Contributor

coliu-akamai commented Mar 3, 2026

just a heads up - I noticed a bug when creating a Linode from backups, where the interface selection is incorrectly disabled. The bug is unrelated to this PR (saw it while testing the firewalls stuff though)

relevant works:

  • M3-10532 (see here)
  • M3-10033 (see here)
  • guessing UIE-9860 (see here) might have helped surface this issue after Linode interfaces became the default selected option? or at least made it more visible than before - based on the code changes in M3-10532 and 10033, this issue probably already existed if users had the Linode Interface Only account setting

Steps to reproduce:

For all account setting options except for Configuration Profile Interface only:

  • Navigate to the the Create Linode from Backups page. Before selecting a Linode, note the interface generation type is correct (linode)
  • Select a Linode with a backup - notice how interface generation type becomes undefined
  • Networking Connection type is disabled (even though selected interface type is Linode in the UI, underlying value is undefined)

For an account setting with Configuration Profile Interface only:

  • Navigate to the the Create Linode from Backups page. Before selecting a Linode, note the interface generation type is correct (legacy_config)
  • Select a Linode with a backup - notice how interface generation type becomes undefined
  • Networking Connection type is disabled but
  • Network Interface type is incorrectly set to Linode even though it should be Configuration Profiles - this is because if interface generation type is undefined, Linode is now always chosen

I think I have a fix here, just testing a few more things

See video to view behavior:

linode-backups-behavior.mov

Copy link
Contributor

@jdamore-linode jdamore-linode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdamore-linode - can you test the changes in this PR and also take a look at e2e test cases that are failing in this PR? I'm guessing failing ones are not related to changes in this PR. Test report is somehow not generating after pushing commits.

Thanks Ganesh, test changes all look good and I confirmed that the other E2E failures are unrelated and can be ignored for the PR. I'll keep an eye on this and will test it once it's merged to develop 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Linodes Dealing with the Linodes section of the app Ready for Review

Projects

Status: Approved

Development

Successfully merging this pull request may close these issues.

7 participants